Skip to content

TIMX 515 - static DuckDB file prep#154

Merged
ghukill merged 3 commits into
mainfrom
TIMX-515-static-duckdb-file-prep
Jul 14, 2025
Merged

TIMX 515 - static DuckDB file prep#154
ghukill merged 3 commits into
mainfrom
TIMX-515-static-duckdb-file-prep

Conversation

@ghukill

@ghukill ghukill commented Jul 11, 2025

Copy link
Copy Markdown
Contributor

Purpose and background context

This PR adds support for a local MinIO S3 instance as prep work for the more involved work in TIMX-515.

As we begin to explore the management of a static file in S3, it will be handy to have easy pathways for more local dev and testing while still getting the constraints and edge cases of AWS S3.

Additionally, renamed the attached TIMDEXDatasetMetadata instance on TIMDEXDataset to just .metadata, inline with potentially more common and frequent usage.

There is no dedicated ticket for this work, just some prep work.

How can a reviewer manually see the effects of these changes?

1- Set the following env vars:

MINIO_S3_ENDPOINT_URL=http://localhost:9000
MINIO_USERNAME="admin"
MINIO_PASSWORD="password"
MINIO_DATA=/tmp/tda-minio

2- Run MinIO server and create structure:

make minio-start
  • navigate to http://localhost:9001 and login with credentials admin / password
  • create a bucket called timdex

3- Start ipython shell:

pipenv run ipython

4- Write some data to the empty dataset, confirming writes to MinIO work:

from timdex_dataset_api import TIMDEXDataset
from tests.utils import generate_sample_records

td = TIMDEXDataset("s3://timdex/dataset")

writes = [
    generate_sample_records(100, source='alma', run_type='full', run_date='2025-07-01', timdex_record_id_prefix='alma'),
    generate_sample_records(50, source='alma', run_date='2025-07-02', timdex_record_id_prefix='alma'),
    generate_sample_records(50, source='libguides', run_type='full', run_date='2025-07-11', timdex_record_id_prefix='libguies'),
]
for records in writes:
    td.write(records)

5- Reload the dataset with current records only, triggering the use of TIMDEXDatasetMetadata and run a couple of metadata queries, demonstrating that reading files from MinIO works for pyarrow for TIMDEXDataset loading and via DuckDB for TIMDEXDatasetMetadata queries:

from timdex_dataset_api import TIMDEXDataset
from  timdex_dataset_api.config import configure_dev_logger

configure_dev_logger()

td = TIMDEXDataset("s3://timdex/dataset")
td.load(current_records=True)
"""
INFO:timdex_dataset_api.metadata:configuring S3 connection
INFO:timdex_dataset_api.metadata:creating table of full dataset metadata
INFO:timdex_dataset_api.metadata:'records' table created - rows: 200, elapsed: 0.04554949980229139
INFO:timdex_dataset_api.metadata:creating view of current records metadata
INFO:timdex_dataset_api.metadata:'current_records' view created - rows: 150, elapsed: 0.007222665939480066
INFO:timdex_dataset_api.metadata:metadata database setup elapsed: 0.09048424987122416, path: ':memory:'
INFO:timdex_dataset_api.dataset:Dataset successfully loaded: 's3://timdex/dataset', 0.11s
"""

td.metadata.conn.query("""select source, count(*) from records group by source;""")
"""
Out[3]: 
┌───────────┬──────────────┐
│  source   │ count_star() │
│  varchar  │    int64     │
├───────────┼──────────────┤
│ alma      │          150 │
│ libguides │           50 │
└───────────┴──────────────┘
"""

td.metadata.conn.query("""select source, count(*) from current_records group by source;""")
"""
Out[4]: 
┌───────────┬──────────────┐
│  source   │ count_star() │
│  varchar  │    int64     │
├───────────┼──────────────┤
│ alma      │          100 │ <------- 50 fewer records because not all current
│ libguides │           50 │
└───────────┴──────────────┘
"""

These operations demonstrate that the MinIO S3 instance is successfully used for read/write operations, from both pyarrow and DuckDB.

Includes new or updated dependencies?

NO

Changes expectations for external applications?

NO

What are the relevant tickets?

ghukill added 2 commits July 11, 2025 17:00
Why these changes are being introduced:

As we prepare for expanding the role of dataset metadata,
which may be stored as a static file in S3 next to the dataset,
it will be helpful to test and develop against a realistic S3
backdrop but do so locally.

How this addresses that need:

Configures pyarrow and DuckDB to create appropriate connections
when a 'MINIO_S3_ENDPOINT_URL' env var is present.

Side effects of this change:
* None

Relevant ticket(s):
* https://mitlibraries.atlassian.net/browse/TIMX-515
@ghukill ghukill force-pushed the TIMX-515-static-duckdb-file-prep branch from 7966652 to e1fb022 Compare July 11, 2025 21:01
@coveralls

coveralls commented Jul 11, 2025

Copy link
Copy Markdown

Pull Request Test Coverage Report for Build 16269288984

Details

  • 12 of 14 (85.71%) changed or added relevant lines in 2 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.6%) to 96.037%

Changes Missing Coverage Covered Lines Changed/Added Lines %
timdex_dataset_api/dataset.py 5 6 83.33%
timdex_dataset_api/metadata.py 7 8 87.5%
Totals Coverage Status
Change from base Build 16125849860: -0.6%
Covered Lines: 315
Relevant Lines: 328

💛 - Coveralls

@ghukill ghukill marked this pull request as ready for review July 11, 2025 21:10
@ghukill ghukill requested a review from a team July 11, 2025 21:10
@jonavellecuerdo jonavellecuerdo self-assigned this Jul 14, 2025
Why these changes are being introduced:

While PRAGMA configurations worked for direct parquet file
reading in a local MinIO S3 instance, it did not work for
DuckDB ATTACH statements.

How this addresses that need:

By using a DuckDB secret, setting the MinIO S3 endpoint
and credentials there, other statments like ATTACH work
as expected.

This is overall a better approach anyways!  The PRAGMA
approach seemed like the only option originally, but this
edge case of ATTACH forced revisiting it and this use of
secrets does in fact work.

Side effects of this change:
* None

Relevant ticket(s):
* https://mitlibraries.atlassian.net/browse/TIMX-515
@ghukill ghukill merged commit 9bf9b1b into main Jul 14, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants